-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Delete the object held by gROOT after we reset the globals. #13463
Conversation
Starting build on |
Build failed on ROOT-ubuntu2004/python3. Failing tests: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix makes a lot of sense 👍! I guess it actually fixes #13462, right? Also, this should probably be backported to v6-28-00-patches
.
Starting build on |
Starting build on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just some minor fixes on the comments! I think we want this in v6-28-00-patches
too.
(I think this shouldn't break anything, but let's also wait for the CI.)
We might want to task CMS to test this one, just to check if it fixes #13429... |
Build failed on mac11/noimt. Failing tests: |
Build failed on windows10/default. Failing tests: |
@hahnjo let's wait until we merge the latest ROOT update in cms-sw/cmsdist#8650 (hopefully in a couple of hours) and I will start the testing. Thanks! |
@pcanal can we have a test for this? This sounds pretty terrible to get wrong... |
For example this allows unique_ptr to be deleted first and thus inform TROOT if need be. Fix root-project#13462
Starting build on |
ping? Unless I'm blind, I don't think we have a test for this... |
Not yet :) |
It is at root-project/roottest#1002 |
@aandvalenzuela Did you get a chance to test? |
Yes! I tested it at cms-sw#186, but unit test failures are still present |
Thanks. |
For example this allows unique_ptr to be deleted first and thus inform TROOT if need be
Fix #13462